fix(checker): allow unknown[] as valid mixin constructor rest param type#63362
fix(checker): allow unknown[] as valid mixin constructor rest param type#633620xkhass wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
This PR updates the checker’s “mixin constructor” validation to treat unknown[] the same as any[] for the single rest-parameter requirement, and adds a conformance test + baselines intended to cover the regression from #29707.
Changes:
- Extend
isMixinConstructorTypeto accept rest parameter element typeunknownin addition toany. - Add new conformance test
mixinWithUnknownRestParam.ts. - Add reference baselines for the new test.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/compiler/checker.ts |
Broadens mixin-constructor detection to accept unknown[] rest parameter element type. |
tests/cases/conformance/classes/mixinWithUnknownRestParam.ts |
New conformance test intended to cover unknown[] mixin constructor patterns. |
tests/baselines/reference/mixinWithUnknownRestParam.types |
Types baseline for the new conformance test. |
tests/baselines/reference/mixinWithUnknownRestParam.symbols |
Symbols baseline for the new conformance test. |
tests/baselines/reference/mixinWithUnknownRestParam.js |
Emit baseline for the new conformance test. |
tests/baselines/reference/mixinWithUnknownRestParam.errors.txt |
Error baseline currently capturing a TS2345 failure in the new test. |
| class Base { | ||
| constructor(public x: number) {} | ||
| } | ||
|
|
||
| const Mixed = mixin(Base) |
There was a problem hiding this comment.
The new test currently fails with TS2345 at the call site because Base (constructor (x: number)) is not assignable to new (...args: unknown[]) => {} under --strict. This introduces an unrelated error and prevents the test from cleanly validating the intended mixin/constructor behavior. Consider removing the mixin(Base) usage or changing Base to satisfy the unknown[] constructor constraint so the test exercises only the targeted regression.
| mixinWithUnknownRestParam.ts(14,21): error TS2345: Argument of type 'typeof Base' is not assignable to parameter of type 'ClassConstructor'. | ||
| Types of construct signatures are incompatible. | ||
| Type 'new (x: number) => Base' is not assignable to type 'new (...args: unknown[]) => {}'. | ||
| Types of parameters 'x' and 'args' are incompatible. | ||
| Type 'unknown' is not assignable to type 'number'. |
There was a problem hiding this comment.
This errors baseline asserts TS2345 about typeof Base not being assignable to ClassConstructor, which is unrelated to the PR’s stated goal (accepting unknown[] for mixin constructors) and will remain even after the checker change. If the intent is a positive conformance test for the mixin rule change, the test should be adjusted to compile without errors and the .errors.txt baseline removed/regenerated accordingly.
| const elementType = getElementTypeOfArrayType(paramType) | ||
| return isTypeAny(paramType) | ||
| || elementType === anyType | ||
| || elementType === unknownType | ||
| } |
There was a problem hiding this comment.
isMixinConstructorType now unconditionally calls getElementTypeOfArrayType(paramType), whereas the previous expression only computed the element type when paramType wasn’t any. Since this check can run in class-checking hot paths, consider preserving the short-circuit (e.g. early-return when isTypeAny(paramType) is true). Also, this hunk introduces trailing whitespace and omits semicolons in a file that otherwise consistently uses them; please align formatting, and update the nearby comment that still says the rest parameter must be any[] now that unknown[] is accepted too.
|
See https://github.com/microsoft/TypeScript?tab=readme-ov-file#contribute - bugfixes should be in typescript-go repo |
Problem
Closes #29707
Mixin classes with a constructor using
unknown[]as the rest parametertype were incorrectly rejected with error TS2545:
Since
unknown[]is strictly safer than 'any[]' (it doesn't disable type checking), there's no reason to reject it.Fix
unknown[]as a valid rest parameter type for mixin constructorsmixinWithUnknownRestParam.tswith baselinessTesting
All 124 mixin-related tests pass after this change.